Add missing channel_wise parameter to RandScaleIntensityFixedMean#8741
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds channel-wise support to RandScaleIntensityFixedMean (array) and its dictionary wrapper. The array transform gains a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
738a3f9 to
da88dbe
Compare
…oject-MONAI#8363) The channel_wise parameter was documented in the docstring but not actually accepted by RandScaleIntensityFixedMean or its dictionary variant. This adds the parameter with per-channel random factor generation and scaling, matching the existing pattern in RandScaleIntensity. Also fixes docstring indentation. Signed-off-by: Mohamed Salah <eng.mohamed.tawab@gmail.com>
da88dbe to
f996a93
Compare
ericspod
left a comment
There was a problem hiding this comment.
Hi @engmohamedsalah I made a few comments below but it otherwise looks good to me with a few changes. Thanks!
… simplify loop - Move channel_wise parameter to end of argument list in both array and dictionary variants to preserve positional argument compatibility - Use R.uniform(size=) instead of list comprehension for per-channel factors - Simplify channel-wise __call__ using float() cast, torch.cat, and removing redundant [0] index - Reorder docstrings to match new argument order Signed-off-by: Mohamed Salah <eng.mohamed.tawab@gmail.com>
…nd cat - Add type: ignore[index] for self.factor[i] since mypy can't narrow the type in the channel_wise branch - Add type: ignore[arg-type] for scale_trans output appended to list - Type out list as list[torch.Tensor] to satisfy torch.cat signature - Follows same pattern as existing RandScaleIntensity (line 738-740) Signed-off-by: Mohamed Salah <eng.mohamed.tawab@gmail.com>
|
@ericspod please check again after apply all your comments |
Summary
channel_wiseparameter toRandScaleIntensityFixedMeanandRandScaleIntensityFixedMeand, which was documented in docstrings but never implemented.channel_wise=True, a separate random scale factor is generated per channel, andpreserve_range/fixed_meanare applied per channel — following the existing pattern fromRandScaleIntensity.channel_wisein both array and dictionary transforms.Fixes #8363
Changes
monai/transforms/intensity/array.pychannel_wiseparameter toRandScaleIntensityFixedMean.__init__randomize()to generate per-channel factors whenchannel_wise=True__call__to apply per-channel scaling with individual random factorsmonai/transforms/intensity/dictionary.pychannel_wiseparameter toRandScaleIntensityFixedMeand.__init____call__to pass image data torandomize()(needed for channel count), following the pattern fromRandScaleIntensitydTests
test_channel_wiseandtest_channel_wise_preserve_rangetotest_rand_scale_intensity_fixed_mean.pytest_channel_wisetotest_rand_scale_intensity_fixed_meand.pyTest plan
channel_wisetests pass for both array and dictionary transformsfixed_mean=Truepreserve_rangeclipping works per channel